Skip to content

feat(retrieval): assemble auto-recall context server-side via /search mode="context" - #3534

Merged
ZaynJarvis merged 18 commits into
volcengine:mainfrom
t0saki:feat/search-context-assembly
Aug 5, 2026
Merged

feat(retrieval): assemble auto-recall context server-side via /search mode="context"#3534
ZaynJarvis merged 18 commits into
volcengine:mainfrom
t0saki:feat/search-context-assembly

Conversation

@t0saki

@t0saki t0saki commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Description

Auto-recall assembly lived in every harness plugin: each one searched per memory type, read hits back one by one, and stitched a context block with its own budget and degradation rules. The implementations drifted apart, and the shared weaknesses were visible in production injections — roughly half of the entries degraded to a bare URI plus a score, character budgets distorted up to 6x on CJK text, and adjacent turns re-injected the same memories.

This PR moves assembly into the server as one round trip. /find stays an unchanged stateless primitive, /search gains mode="context" (mode="list" remains the default and is byte-identical to before), and /recall becomes a thin preset over the same kernel with its v1 field names folded onto the new contract.

Implements RFC #3372.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Implements the contract proposed in discussion #3372.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

The breaking part is scoped to the /recall response body: entries now use category/detail/text instead of type/mode/content/summary, rendered is flat XML instead of three levels of nesting, and rank is gone. Request compatibility is preserved — v1 fields are still accepted as aliases on /recall.

Changes Made

  • New assembly kernel under openviking/retrieve/context_assembler/: candidate gathering, tier resolution, token budgeting, flat rendering, dedup ledger, query expansion, digest rewrite. Replaces openviking/retrieve/type_quota_recall.py.
  • Token budgeting with a CJK-aware estimate replaces the character budget. max_tokens is the single budget parameter.
  • Tiers come from a per-category default table rather than one global strategy. events is served at overview and may deepen to full on leftover budget; every other category is served at abstract. detail pins every entry to one tier instead of only capping it, and additionally accepts a per-category map such as {"events":"overview","preferences":"abstract"}. See the tier model note below for why the table looks the way it does.
  • Filling is breadth-first then depth: every candidate lands on its default tier first, then leftover budget deepens in score order. An oversized tier falls back to the previous one instead of being truncated, bounded by max_tokens / candidates * 2 per entry.
  • Overview extraction dispatches by source type: memory files use their leading Summary section, code files use the current code-skeleton extraction API, and long documents use a heading tree plus first paragraph.
  • Directory hits start at the overview tier and read their .overview.md sidecar, since directories carry no stored abstract; their full tier stays capped at overview. Recall v1 injected that sidecar as if it were a whole file.
  • Quotas generalize beyond memory types to resources and skills; purpose presets supply ratios when quotas are absent.
  • dedup_turns keeps a per-session ledger at {session_uri}/.recall_log.json, so every harness inherits cross-turn dedup. exclude_uris remains as the stateless fallback.
  • Every tier carries its URI, so the model can always drill down through the MCP read tool.
  • Server-side query expansion and digest rewriting are optional and fail closed: both have timeout fuses (5s for expansion, 30s for rewrite), and a failed rewrite still returns the unrewritten block. Retrieval failures are reported in stats.retrieval_errors rather than silently yielding an empty block.
  • /recall folds max_charsmax_tokens, min_scorescore_threshold, and the render tri-state → detail, and signals deprecation through a Deprecation header plus stats.deprecated. The MCP recall tool routes through the same kernel.
  • Plugins send one context request and fall back to /recall, then to raw find, on older deployments, caching that outcome so only the first turn pays for the probe. Claude Code and Codex share OPENVIKING_RECALL_COMPRESS / plugin.recallCompress, defaulting to auto: Claude Code prefers local claude -p (Sonnet, low effort) and falls back to server rewrite, while Codex uses local codex exec with Spark then Luna. off disables compression for latency-sensitive paths. Codex also injects profile context at session start through the shared profile builder.
  • A context request that asks the server for a digest gets a deadline that outlasts the server's rewrite fuse, since aborting inside the fuse discards the whole response rather than just the digest. OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS / plugin.recallContextTimeoutMs pins it; the non-rewrite path keeps the ordinary request timeout.
  • plugin joins the ovcli.conf schema in both Python readers, alongside the fields the Rust CLI already writes that had drifted out of them.
  • Docs: mode="context" reference in docs/{zh,en}/api/06-retrieval.md, /recall deprecation and alias table in 16-memory.md, the two retrieval timeouts in the configuration guide, and centralized low-latency plugin settings in the Agent integration overview — including the context-request deadline — with links from the Claude Code and Codex integration pages and image-doc mirrors.

Tier model: why the defaults are per category

The first revision of the ladder assumed abstract is a cheap summary. For memory files it is not. memory_updater.py writes the whole stripped body into the vector row's abstract scalar, because that same field doubles as the embedding text — embedding the full body is the right call on the write side, but it means the ladder is uri < overview < abstract = full for memory, not the strictly monotonic cost ladder the RFC assumed. Only memory is affected: a resource's abstract is the 256-char summary semantic processing produces, and directories have real .abstract.md / .overview.md sidecars.

Two properties of the first revision fell out of that mismatch:

  • abstract was exempted from the per-entry cap on the grounds that it is "cheap by construction", which let a single memory entry consume several times the per-entry budget.
  • detail only ever set a ceiling, so auto and full produced byte-identical output for a memory-only candidate set, and in the 0.38–0.50 score band the RFC itself reports, auto, overview and full were all indistinguishable.

Measured over a real memory store (~2100 files), events is the only memory type whose # Summary extraction is a real compression (median 259 tok body → 66 tok overview, 75% saved); entities and preferences have a median body of ~76 tok, where an overview costs a file read to return a truncated version of something already in hand. So the defaults exploit the storage shape rather than fight it: events starts at overview, everything else is served from abstract, and only events can deepen. The table carries the note to move events back to abstract once the writer stores a separate summary scalar — that fix belongs in the writer and is deliberately not attempted here, since abstract cannot be changed without changing recall quality.

Two consequences worth calling out: the default path now reads only the events candidates instead of every hit, and since the cap exemption is gone, an oversized abstract falls back to overview before it falls back to a bare URI.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

OPENVIKING_CONFIG_FILE=/tmp/ov-test.conf uv run pytest tests/retrieve tests/server/test_api_search_context.py tests/server/test_recall_endpoint.py tests/server/test_recall_peer_scope.py tests/server/test_mcp_endpoint.py tests/test_ovcli_config_schema.py — 124 passed, with /tmp/ov-test.conf containing {}. New coverage spans candidate gathering, tier dispatch, budget filling and fall-back, the dedup ledger, expansion and rewrite failure modes, the 400 validation matrix, and /recall alias folding. Two of the new server tests go through real AGFS: one reads file bodies and directory sidecars, the other round-trips the dedup ledger against a real session. tests/test_ovcli_config_schema.py asserts the shipped ovcli.conf.example loads in both Python readers and that an unknown field is still rejected, so the schema cannot drift away from the Rust CLI again unnoticed.

The tier-model tests pin the per-category defaults, the pin semantics of an explicit detail, the per-category map, the read gating (only events candidates are read on the default path), the oversized-abstract fallback, and the degradation of an unknown detail value. The bug fixes below each have a regression test, including one that asserts rewrite_usage is dropped when the shared tracker moved by more than one call — the previous test mocked a planner shape that does not exist in production, which is how the dead path stayed green.

OPENVIKING_STATE_DIR="$(mktemp -d)" node --test $(rg --files examples | rg '\.test\.mjs$' | sort) — 193 passed, covering the context-face request body, the downgrade chain, the legacy-server cache, the unified compression knob and model fallback matrix, the context-request deadline, session-start profile injection, and digest URI repair.

cd docs && npm run check:api — passes, which is what the previous revision broke.

Also verified against a locally running server: the context response contract, all four 400 validation cases, /recall alias folding (max_chars: 6500max_tokens: 1625, defaults of 1600/0.35, render: "compact" → abstract ceiling), the Deprecation and Link headers, and a real Claude Code hook run that reached the context face and degraded gracefully when retrieval was unavailable.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

Bugs found while reworking the tier model, each fixed with a regression test:

  1. Rewrite timeouts were reported as failed on Python 3.10, where asyncio.TimeoutError is a separate class from the builtin. requires-python is >=3.10 and release wheels are built there; CI only runs 3.11, so the assertion never fired.
  2. stats.rewrite_usage read token_tracker off VLMConfig, which has no such attribute — the AttributeError was swallowed and usage was structurally always null, which quietly disables the cost accounting RFC §3.2 promises. It now reads the model instance's tracker and reports only when the call count moved by exactly one, because that tracker is shared across callers.
  3. A single malformed ledger record ({"turn": "x"}, a null turn, a non-dict value) made every deduped recall in that session fail after the full retrieve-read-budget-render pass, and the file was never rewritten, so it could not heal. Records are coerced on read and dropped on the next write, together with records left ahead of the clock by an archive rotation, which previously could never expire and additionally won the eviction sort.
  4. Entries served as a bare URI no longer enter the dedup cooldown — they lost to budget pressure, not to the reader having already seen them. This is the uri-grace behaviour the RFC lists as a benefit of server-side accounting.
  5. The render envelope only neutralised a literal </memory>, so a body could emit <memory uri="..." score="0.99">…</Memory> and forge a sibling entry with its own provenance. Both ends of the tag are now neutralised, case- and whitespace-tolerantly.
  6. Flat-mode gathering discarded the owning bucket and re-derived the category from the URI, so viking://resources/backup/memories/events/log.md was read as an event and escaped the resource tier ceiling.
  7. Cooled and excluded URIs are filtered after retrieval, so a bucket whose whole top page was cooled came back empty rather than falling through to the next-best hits. Retrieval now asks for compensating rows.
  8. /recall quotas overlay the v1 bucket defaults again. v1's normalize_quotas merged over the defaults; the rewrite started from an empty map, so {"events": 5} silently dropped the other three buckets and {} returned nothing at all.
  9. The MCP recall tool sent its own signature defaults as if the caller had supplied them, so its default profile resolved to 0.1/1625 while POST /recall resolved to 0.35/1600 — RFC §3.1 requires the same profile. An unknown detail value ("summary", the v1 spelling an LLM readily produces) also raised KeyError through the whole call instead of degrading.

Found in the second review round:

  1. Build Docs was failing. The API overview's deprecated-recall row described the successor as `/search`, and docs/scripts/check-api-reference.mjs scans everything after the method cell for backticked paths, so it read that description as a route named POST /search — a route the server does not mount and no reference page documents. Both locales now name the endpoint without backticks.
  2. Adding the documented plugin section to a working ~/.openviking/ovcli.conf broke every Python consumer of that file: load_ovcli_config() raised Unknown field 'ovcli.plugin' and OVCLIConfig raised extra_forbidden, so ov doctor and SDK client construction failed before any request went out. The deeper cause is that ovcli.conf's schema belongs to the Rust CLI, which writes root_api_key, output, echo_command, show_progress and verbose and ignores unknown keys, while the two Python readers had each drifted into a different stricter subset — examples/ovcli.conf.example already failed to load in both on main, before this PR. Both readers now accept the full field set, and a test pins the example against them.
  3. mode="context" answered an invalid request with 200 and an empty block. Retrieval validates query and image_url and raises InvalidArgumentError before searching; the gather fuse caught it alongside genuine per-scope failures, so {"mode":"context"} recorded a stats.retrieval_errors entry and returned success where mode="list" returns 400. That contradicts the documented "L0 parameter behaviour matches list mode" and left callers unable to tell a malformed request from a genuine miss. Request rejections now propagate; runtime failures still degrade.
  4. With recallCompress=server — or auto when no local compressor is available — the plugin sent a context request under the ordinary 15s HTTP timeout while the server's rewrite fuse is 30s. A rewrite finishing at 20s was inside its own budget but aborted client-side, and since the abort fails the whole request the plugin fell back to /recall, losing the uncompressed rendered block the server returns even when a rewrite fails. The deadline now outlasts the fuse, but only when the body actually requests a rewrite, and OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS / plugin.recallContextTimeoutMs pins it for deployments that tune retrieval.recall_rewrite_timeout_s.

… mode="context"

Auto-recall assembly lived in every harness plugin: each one searched per
memory type, read hits back one by one, and stitched a context block with its
own budget and degradation rules. The implementations drifted, and the shared
weaknesses showed up in production injections — roughly half of the entries
degraded to a bare URI plus a score, character budgets distorted up to 6x on
CJK text, and adjacent turns re-injected the same memories.

This moves assembly into the server as one round trip. /find stays an unchanged
stateless primitive. /search gains mode="context" (mode="list" is the default
and byte-identical to before), and /recall becomes a thin preset over the same
kernel with its v1 field names folded onto the new contract.

New assembly kernel under openviking/retrieve/context_assembler/:

- Token budgeting with a CJK-aware estimate replaces the character budget.
- detail="auto" fills breadth-first then deepens: every candidate gets a
  readable floor, then overview, then full for high-scoring entries. An
  oversized tier falls back to the previous one instead of being truncated,
  bounded by max_tokens / candidates * 2 per entry.
- Overview extraction dispatches by source: memory files use their leading
  Summary section, code files reuse code_outline signatures, long documents use
  a heading tree plus first paragraph.
- Directory hits start at overview and read their .overview.md sidecar, since
  directories carry no stored abstract; their full tier stays capped at
  overview. v1 injected the sidecar as if it were a whole file.
- Quotas generalize beyond memory types to resources and skills, with purpose
  presets supplying ratios when quotas are absent.
- dedup_turns keeps a per-session ledger at {session_uri}/.recall_log.json so
  every harness inherits cross-turn dedup; exclude_uris remains as the
  stateless fallback.
- Rendering flattens to one <memory uri=... type=... score=... detail=...>
  element per entry. Every tier carries its URI, so the model can always drill
  down through the MCP read tool.
- Query expansion and digest rewriting are opt-in and fail closed: both have
  timeout fuses, and a failed rewrite still returns the unrewritten block.
  Retrieval failures are counted into stats rather than silently yielding an
  empty block.

Plugins now send one context request, falling back to /recall and then to raw
find on older deployments, and cache that outcome so only the first turn pays
for the probe. The tri-state recallRewrite knob chooses between local host-CLI
compression and the server digest, and client-side settings move to a plugin
section in ovcli.conf.
The tier ladder assumed `abstract` is a cheap summary. For memory files it
is not: the memory writer stores the whole stripped body in that scalar
because it doubles as the embedding text, so `abstract` costs the same as
`full` and the ladder runs `uri < overview < abstract = full`. Two of the
model's properties fell out of that: exempting `abstract` from the per-entry
cap let a single entry eat several times the budget, and `detail` — which
only ever set a ceiling — collapsed to two distinguishable behaviours across
its four values, since `auto` already allowed `full` for memory.

Tiers now come from a per-category constant table that treats the storage
shape as a given: `events` starts at overview (the one memory type whose
`# Summary` extraction is a real compression) and may deepen to full on
leftover budget; every other category is served at `abstract`, which for
memory already is the complete file at zero read cost and for resources and
skills is the generated 256-char summary. The table carries the note to move
`events` back to `abstract` once the writer stores a separate summary scalar.

Falling out of that: prefetch now reads only the candidates whose planned
tier needs a body rather than every candidate, `detail` becomes a real pin
(start and ceiling) and additionally accepts a per-category map, and
`full_score_threshold` is gone — leftover budget is spent in score order
instead of behind an absolute threshold the observed score band cannot
support. `auto` is still accepted on the wire as a synonym for "unset".

Assembly fixes found alongside:

- Removing the abstract cap exemption would turn an oversized abstract into
  a bare URI, so it now falls back to overview first — for memory that is a
  cheaper substitute, not a step up.
- Rewrite timeouts were reported as failures on Python 3.10, where
  `asyncio.TimeoutError` is a separate class from the builtin.
- `stats.rewrite_usage` read `token_tracker` off `VLMConfig`, which has no
  such attribute; usage was structurally always null. It now reads the model
  instance's tracker and reports only when the call count moved by exactly
  one, since that tracker is shared.
- A single malformed ledger record made every deduped recall in that session
  fail, and the file was never rewritten, so it could not heal. Records are
  now coerced on read and dropped on the next write, along with records left
  ahead of the clock by an archive rotation.
- Entries served as a bare URI no longer enter the dedup cooldown: they lost
  to budget pressure, not to the reader having already seen them.
- The render envelope only neutralised a literal `</memory>`, so a body could
  forge a sibling entry with its own uri, type and score.
- Flat-mode gathering re-derived the category from the URI, reading
  `viking://resources/backup/memories/events/log.md` as an event.
- Cooled and excluded URIs are compensated with extra rows, so a fully cooled
  bucket falls through to the next-best hits instead of coming back empty.
- `/recall` quotas overlay the v1 bucket defaults again; `{"events": 5}` had
  started dropping the other three buckets.
- The MCP `recall` signature sent its own defaults as if the caller had, which
  resolved a different profile than `POST /recall`; an unknown `detail` value
  raised `KeyError` through the whole call instead of degrading.
huangruiteng added a commit that referenced this pull request Jul 30, 2026
- events.yaml: remove hardcoded ratio_threshold=0 so summaries are
  preferred over full transcripts when short enough
- memory_updater: skip empty-speaker lines for turns with no text
  content in _format_contiguous_group

Part of #3598 (write-path residuals, does not overlap with #3534)
@t0saki
t0saki marked this pull request as ready for review July 31, 2026 09:24
Copilot AI review requested due to automatic review settings July 31, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

t0saki added 2 commits July 31, 2026 17:37
…ssembly

# Conflicts:
#	openviking/server/mcp_endpoint.py
#	openviking_cli/utils/config/retrieval_config.py
Comment thread examples/memory-plugin-shared/lib/recall-compress-core.mjs Outdated
Comment thread openviking/retrieve/context_assembler/pipeline.py Outdated
Comment thread openviking/retrieve/context_assembler/gather.py Outdated
Comment thread openviking/retrieve/context_assembler/gather.py Outdated
Comment thread openviking/retrieve/context_assembler/rewrite.py Outdated
Comment thread openviking/retrieve/context_assembler/recall_preset.py Outdated

@qin-ctx qin-ctx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server-side context assembly is the right ownership move: before this PR, each plugin had to issue multiple retrieval/read calls and assemble its own bounded context; after this PR, /search with mode="context" centralizes retrieval, tiering, budgeting, deduplication, and optional rewrite while keeping /find as the primitive path.

I am requesting changes because five supported paths currently violate the documented retrieval/configuration contracts: local digest reuse is not query-safe, context expansion ignores enable_intent=false, flat retrieval does not honor peer_scope="all", bucketed retrieval drops image_url, and server rewrite can return citations outside the served entry set. I also left one non-blocking inline comment on the /recall successor metadata.

t0saki and others added 3 commits August 3, 2026 12:22
Co-authored-by: TRAE CLI <noreply@bytedance.com>
Resolve retrieval conflicts while preserving the context assembler migration and streamlined test coverage.

Co-authored-by: TRAE CLI <noreply@bytedance.com>
qin-ctx
qin-ctx previously requested changes Aug 3, 2026

@qin-ctx qin-ctx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

服务端集中组装 Context 的方向合理,之前 review 提出的缓存键、enable_intent、peer scope、图片检索和引用约束等问题也已确认修复。当前仍有两个需要在合并前处理的问题:新增的 ovcli.conf.plugin 与现有 Python SDK/CLI 的严格配置 schema 不兼容,以及中英文 API overview 触发了文档检查失败。另有两条非阻塞意见,分别涉及 context 模式的请求错误分类和 Claude Code 服务端重写的超时边界。

Comment thread examples/ovcli.conf.example
Comment thread docs/en/api/01-overview.md Outdated
Comment thread openviking/retrieve/context_assembler/gather.py
Comment thread examples/claude-code-memory-plugin/scripts/config.mjs
t0saki and others added 5 commits August 4, 2026 12:03
- Drop the backticked `/search` from the deprecated-recall row in both API
  overviews. The reference checker scans the whole row after the method cell
  for backticked paths, so it read the description as a route named
  `POST /search` and Build Docs failed on an unknown, undocumented route.
- Accept ovcli.conf's full field set in both Python readers. The file's schema
  belongs to the Rust CLI, which writes `root_api_key`, `output`,
  `echo_command`, `show_progress` and `verbose` and ignores unknown keys; the
  two Python readers had drifted into stricter subsets, so the shipped example
  already failed to load in both. Adding the new `plugin` section to a working
  ovcli.conf would have broken `ov doctor` and every SDK client the same way.
- Return 400 from `mode="context"` for a request `mode="list"` also rejects.
  Retrieval validates query and image_url before searching, and the gather
  fuse swallowed that rejection along with genuine scope failures, so a body
  of `{"mode":"context"}` came back 200 with an empty block instead of the
  documented parameter error. Runtime failures still degrade into
  `stats.retrieval_errors`.
- Let a context request that asks for a server-side digest outlast the
  server's rewrite fuse. The plugin's ordinary 15s request timeout is shorter
  than the 30s fuse, so a rewrite that finished inside its own budget was
  aborted client-side, discarding the whole response — including the
  uncompressed block the server returns when a rewrite fails — and falling
  back to `/recall`. The deadline is only extended when the body actually
  requests a rewrite, and `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS` /
  `plugin.recallContextTimeoutMs` pins it.
Restore cross-domain coding recall, reuse authoritative actor resource
scopes, and make bucket quotas the sole width control in purpose mode.
Keep plugin defaults server-owned while preserving explicit legacy limit
settings through quota conversion.

Co-authored-by: TRAE CLI <noreply@bytedance.com>
Restore the deprecated recall threshold default, distinguish successful empty rewrites from compressor failures, and document legacy quota floors across coding-agent plugins.

Co-authored-by: TRAE CLI <noreply@bytedance.com>

@ZaynJarvis ZaynJarvis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all resolved

@ZaynJarvis
ZaynJarvis merged commit 2cc96e3 into volcengine:main Aug 5, 2026
8 of 9 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants